feat(client): validate safe integer range for x-mcp-header parameters (#445) - #499
feat(client): validate safe integer range for x-mcp-header parameters (#445)#499Yudis-bit wants to merge 2 commits into
Conversation
…modelcontextprotocol#445) Per the Streamable HTTP specification (SEP-2243) and server/tools.mdx: 'Integer values MUST be within the safe range for integers represented using IEEE754 double-precision floating point numbers (−2^53+1 to 2^53−1)' Previously, the conformance harness had no requirement row in sep-2243.yaml, no emitted check ID, and no verification that clients refrain from mirroring out-of-range integer values into Mcp-Param headers. 1. Add sep-2243-x-mcp-header-integer-safe-range to src/seps/sep-2243.yaml and CUSTOM_HEADERS_DECLARED_CHECK_IDS in http-custom-headers.ts. 2. Add an annotated integer parameter unsafe_integer_val to test_custom_headers with context argument 9007199254740992 (2^53). 3. In HttpCustomHeadersScenario.handleToolsCall, verify that the client does not mirror unsafe integer arguments into Mcp-Param-UnsafeInteger. 4. Add positive and negative unit test assertions in http-custom-headers.test.ts. 5. Update traceability.json for SEP-2243. Closes modelcontextprotocol#445
|
This will fail csharp-sdk on five checks, and the failure message will name the wrong cause. The oversized integer goes into the existing Five of the six declared IDs are emitted only from the Give the probe its own tool and its own @JosephDoUrden asked for this issue on August 28 and built the same check, then held it pending a maintainer steer (comment). That comment has the four-SDK survey and the case for whether the constraint binds at definition time or at call time. The dedicated probe tool above is their design. CONTRIBUTING asks for a run against at least one real SDK before opening a PR. Validation lists typecheck, lint, build and the unit suite, but no SDK run. The unit tests build the request and its headers by hand, so they cannot catch a client that never sends the call. The bigger question is for the maintainers. Scoring omission as SUCCESS answers the open question in that comment, and makes csharp-sdk non-conforming by merge rather than by decision. |
|
@Danny-Devs Thanks for catching the coupled failures and pointing me to @JosephDoUrden's earlier design and SDK survey. I've pushed the isolation fix in 7482d37.
All 595 tests across 45 files, typecheck, lint, build, and pre-push checks pass. The specification concern remains open. This revision retains the proposed wire-level success result for omission, and a rejecting client still receives an untestable failure. Isolation fixes the misleading collateral failures; it does not resolve definition-time versus call-time enforcement or error versus omission. I've made that limitation explicit in the code and PR description and removed the description's claim to close #445. Maintainer guidance is still needed before merging this as conformance policy. |
2 participants
Related to #445.
An out-of-range integer in the ordinary
test_custom_headerscall can make a client reject that entire call before sending it, incorrectly failing several unrelated header requirements. This change gives the integer-range probe its own tool and places itstoolCallsentry last, after the ordinary encoding and null-handling calls.Changes
sep-2243-x-mcp-header-integer-safe-rangeinsep-2243.yamland the scenario's emitted check IDs.test_custom_headers_unsafe_integerwith an annotated integer argument and request the exactly representable, out-of-safe-range value9007199254740992(2^53). The ordinary tool no longer contains this argument.Mcp-Param-UnsafeInteger; retain the proposed wire-level success result when the requested argument arrives without that header.details.untestable: trueand a diagnostic naming the dedicated tool and possible local rejection. Absence alone cannot distinguish rejection from a skipped call. A missing or changed probe argument also cannot produce a success result.Specification decision still needed
The discussion on #445 raises both definition-time versus call-time enforcement and error versus omission. This implementation exercises the proposed call-time, wire-level range constraint; it does not settle those questions or establish that silent omission is the normative recovery behavior.
A locally rejecting client still gets an untestable failure for the range check, following the harness's missing-prerequisite policy. Its ordinary header checks remain independently testable. Maintainer guidance is still needed before treating this proposal as the final conformance policy. The rejection validation below is a local fixture, not a C# SDK run.
Validation
npm run check— typecheck, ESLint, and Prettier pass.npm run build— passes.npm test— 45 test files, 595 tests pass, including 15 custom-header tests.Real TypeScript SDK at
b65426158ed9f29aea8ef3dc09ca22d7d9d6f970, using its unmodifiedtest/conformance/src/everythingClient.ts, with the SDK packages built from source:node dist/index.js client \ --command "node --import tsx .sdk-under-test/typescript-sdk/test/conformance/src/everythingClient.ts" \ --scenario http-custom-headers --spec-version 2026-07-28 \ -o results/pr-499-typescript19/19 checks pass; client and runner exit 0. The client used Node 22.14.0 on Windows. Node 24.19.0 produced the same 19 passing wire checks but crashed during client shutdown with a libuv assertion, so it is not counted as a passing end-to-end run.
Negative CLI validation with the same SDK and a local fetch wrapper that injects
Mcp-Param-UnsafeInteger: 9007199254740992only for the dedicated probe: 18/19 pass; only the integer-range check fails, runner exit 1.Local-rejection CLI validation with the same SDK and a fetch wrapper that throws before transmitting the dedicated probe: 18/19 pass; only the integer-range check fails, with
untestable: trueand the dedicated-tool diagnostic; client and runner exit 1.AI assistance disclosure: Initial implementation used Antigravity; review fixes and validation used Codex.